-
Notifications
You must be signed in to change notification settings - Fork 71
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(diagnostics): improve assignment fallibility compilation error #553
feat(diagnostics): improve assignment fallibility compilation error #553
Conversation
# │ │ this expression is fallible | ||
# │ │ update the expression to be infallible | ||
# │ │ note if an argument type is invalid it can render a function fallible | ||
# │ │ '.result[0].an' argument type is 'string or undefined' and this function expected a parameter 'value' of type 'string' | ||
# │ or change this to an infallible assignment: | ||
# │ .a, err = sha3(.result[0].an) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# │ │ this expression is fallible because at least one argument's type cannot be verified to be valid
# │ │ '.result[0].an' argument type is 'string or undefined' and this function expected a parameter 'value' of type 'string'
# │ update the expression to be infallible by adding a `!`: `.a = sha3!(.result[0].an)`
# │ or change this to an infallible assignment:
# │ .a, err = sha3(.result[0].an)
What you have here is a great step forward, but do you think we can make it even clearer with something like what I put? I still worry that the current error message will be confusing to users. I'm not sure exactly what's possible though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I read the above correctly you want to add two more lines:
this expression is fallible because at least one argument's type cannot be verified to be valid
// ...
update the expression to be infallible by adding a `!`: `.a = sha3!(.result[0].an)`
which should be doable and I will follow up on this.
Ultimately users should have some basic understanding about fallibility and fail safety when using VRL (which practically means adding a !
or handling the returned err
).
P.S. Where users might still be confused, is with code blocks which contain fallible expressions example here. However, that part of the codebase is not easy to maintain and is need of refactoring.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# │ │ this expression is fallible because at least one argument's type cannot be verified to be valid
# │ │ '.result[0].an' argument type is 'string or undefined' and this function expected a parameter 'value' of type 'string'
is actually the more important update I would like to see. I think my verbiage makes it more clear exactly what the issue is: that the function is fallible because one of the arguments cannot be verified, and this is the argument (or arguments) in particular, their types, and what the function expects.
The hints on how to fix are then a cherry on top.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The hints could also include casting the arguments like:
# │ .a = sha3(string!(.result[0].an))
I think that is closer to the "best-practice" that we want to guide people into.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gotcha, this makes sense.
closes: #23